fix(native): recover unattended sync helper setup - #681

🎨 Storybook preview

Open Storybook preview

Updated for 4e5f802

⚠️ Detected UI changes (5)

These stories' HTML snapshots changed. I've added screenshots + links to the changed stories below. Review them carefully then accept the changes to regenerate baselines and include them in this PR:

Flows/Evolve › Playground

Flows/Evolve › 1. Begin (idle)

Flows/Evolve › 2. Evolving (progress)

Flows/Evolve › Evolving With Error Event

Flows/Evolve › 3. Review (changes generated)

Accept UI changes

• Click here to accept these changes

Alternatively, you can run bun run test:update-snapshots locally to re-generate the baselines and then push the changes to this PR.

📋 PR Overview

🔬 Coverage

Generated by 🚫 dangerJS against 4e5f802

Typed helper lifecycle phases replace grace-period timer and text parsing

Product decisions in this change

1. The backend now owns a typed helper lifecycle with seven named phases (Ready, ApprovalRequired, Reconciling, WaitingForActivation, NeedsUserAction, Failed, Disabled), replacing UI-side parsing of diagnostic prose.

Every surface -- repair banner, permissions panel, feedback snapshot -- previously had to interpret the same diagnostic string independently. Centralising the phase in the backend means the same state drives the same copy and actions everywhere, and copy changes happen in one place. This is the correct ownership boundary: the backend understands what each reconciliation outcome means; the frontend should not re-derive that meaning.

2. The helper banner now appears immediately when the user's opted-in helper is not ready, with a neutral blue spinner for `reconciling` and `waitingForActivation` phases and amber for failure phases.

The old behaviour showed nothing for ten seconds, leaving the user with no signal while the backend was actively working. The new behaviour shows a neutral progress state from the first detection. Users who opted in and see a spinner understand that something is happening. Users who see amber understand that action may be needed. The distinction between neutral and alarming colour is appropriate: a helper that is registering is not a problem yet; a helper that has failed after the display window is.

3. A helper that keeps failing transitions from the neutral `reconciling` state to the `failed` state after ten seconds of failing attempts, not immediately.

Showing reconciling first and failed after a window is the right shape: it suppresses transient failures that self-resolve before the user reads the banner. The concern is that ten seconds remains a constant chosen without production timing data. The user experience is meaningfully better than before -- the user sees a spinner rather than nothing -- but the ten-second threshold could still be wrong in either direction. A fast machine that always fails in under one second waits unnecessarily. A slow registration that normally takes twelve seconds would show failed before it completes.

Option	What users see	Cost	Effort to change later
Current (10 s fixed)	Spinner, then failure	May be too long on fast machines; too short on slow ones	Change one backend constant
P90-derived threshold	Spinner stops near the true tail	Requires timing instrumentation on real hardware	Needs data collection before the constant can change
Immediate failure	No spinner; failure from first attempt	Alarming on healthy installs that just take time	Low effort but wrong UX

4. Retry is a separate action from Enable (Grant). Retry re-executes the stored decision without changing it or opening System Settings. Enable changes the user's opt-in.

A user who opted in and whose helper failed does not need to re-confirm the opt-in to try again. Conflating Retry with Enable would either force a redundant confirmation or silently turn a failed opt-out into a new opt-in. The separation is also correct for the failed-disable case: the permissions panel shows Retry for a failed removal without showing Enable, so the user cannot accidentally re-enable by clicking the recovery button.

5. Dismissing a helper banner clears when the phase changes, not permanently. A later failure re-shows the banner even if the user dismissed an earlier progress notice.

Permanent dismissal was the wrong model: a user who dismissed a reconciling banner should still see failed if recovery does not happen. Keying the dismiss state to the phase makes the dismiss mean "I see this progress state; do not repeat it" rather than "I never want to hear about the helper again." The phase is the identity that matters, not the banner kind.

6. After a Disable action completes, the permissions panel reads the live row, not the RPC response, to decide what to show.

The RPC response described the state at the moment the action ran. The row reflects the state after reconciliation and publication. Showing the RPC response as a notice could outlive the state it described: if activation was running when Disable was called, the response might say "waiting for activation" long after activation finished and the helper was removed. The live row is always current; the RPC result is a snapshot from one moment.

7. A `notFound` response from ServiceManagement no longer automatically means the installation is broken. The app checks the bundled plist and binary first, and registers if they are valid.

macOS's notFound is ambiguous: it means either "this service was never registered" or "the plist this service would use is gone." A valid app on a first install is in the first state, not the second. Treating both as a broken installation blocked first-time users from enabling the helper at all -- an Enable button was never shown. Checking the bundle independently resolves the ambiguity without relying on an unstable framework error code.

8. When a concurrent background check cannot get the reconciliation slot, the last meaningful row is preserved rather than replaced by a generic `reconciling` state.

A Busy result says only that this observation raced and lost. It carries no information about actual helper state. Replacing an actionable instruction such as "approve in Login Items" with a generic spinner would suppress information the user needs. Preserving the last row means the UI shows the most recent real observation until a real observation replaces it.

9. WaitingForActivation shows no user action -- no Retry, no Open Settings. The banner displays a spinner and the backend loops without a bounded wait.

Showing no action while an activation is legitimately in progress is correct: a registration attempt mid-activation would conflict with the running activation. The concern -- unchanged from the prior review -- is that the loop's WaitingOnActivation path does not increment the mutation counter and has no wall-clock timeout. If a darwin-rebuild activation hangs -- for example, an osascript call that blocks waiting for a UI prompt -- the user sees a spinner indefinitely. Dismissing the banner hides it; because dismissal clears on phase change and the phase does not change during a hang, the banner does not return after dismissal. The user is left with a stuck helper and no in-app signal or recovery path. This requires either a bounded wait in the helper loop or a confirmed statement that the activation layer has its own timeout that bounds this phase.

10. The Login Items instruction changes from "Approve nixmac" to "Allow nixmac".

The macOS Login Items & Extensions UI uses the word "Allow", not "Approve". Matching the system vocabulary reduces the chance that a user searches for a button that does not exist.

11. Helper failure diagnostics are included in the opt-in feedback snapshot, redacted of credential-shaped strings.

The failure detail (for example, SMAppServiceErrorDomain 1) is exactly the information the team needs to reproduce and fix real helper failures. The redaction test confirms the pattern removes token-shaped strings before the snapshot is sent. This is the correct tradeoff: diagnostic value with credential safety.

Since the last review

• Addressed in code: Ten-second neutral window before failure escalation is a constant chosen without production timing data. (The frontend HELPER_GRACE_MS = 10_000 timer is removed. The backend FAILURE_PRESENTATION_AFTER: Duration = Duration::from_secs(10) replaces it. The user now sees a reconciling spinner during this window rather than nothing, which lowers the UX cost of an overly long threshold. The constant is still hardcoded but the stakes are lower.)
• Still open: WaitingForActivation loop does not increment the mutation counter, so a hung activation leaves the user with an indefinite spinner and no in-app exit. (The drive loop still does wait(WAITING_INTERVAL); continue; on Reconciled::WaitingOnActivation(_) without incrementing working or checking elapsed time. The new test offers no repair action while waiting for an activation confirms no action is offered, and no timeout is added to this path.)
• Addressed in code: Retry button copy does not distinguish a failed enable from a failed disable, so the user cannot confirm what Retry will do. (The repair banner condition gates on helperPreference === "granted", so "Couldn't finish enabling unattended sync" is contextually correct there. In the permissions panel, a failed disable shows Retry without an Enable button (confirmed by the test retries a failed removal without offering to enable the helper again), preventing accidental re-opt-in even though the copy is generic.)
• Still open: No p90 reconciliation timing data to validate the ten-second neutral window on real hardware. (No timing data or instrumentation is added in this PR. The FAILURE_PRESENTATION_AFTER constant remains Duration::from_secs(10) with no comment citing measurement.)
• Still open: Unknown whether the darwin-rebuild activation layer has its own timeout bounding the WaitingForActivation phase. (No timeout is added to the WaitingOnActivation loop path, and the PR description and diff contain no statement about whether the activation layer provides its own bound.)

Open questions

• What is the p90 reconciliation time on a healthy first install on real hardware? This determines whether the ten-second failure presentation window correctly separates transient failures from persistent ones, or whether it is too short for slow machines and too long for fast ones.

• Does the darwin-rebuild activation layer have its own wall-clock timeout that bounds the WaitingForActivation phase? If it does not, a hung activation leaves the user with an indefinite spinner and no in-app recovery after dismissal.

Recommendation

Ship with changes
The PR fixes a real first-install defect, replaces fragile UI-side text parsing with backend-owned typed phases, and separates Retry from Enable correctly. One prior concern remains fully open: the WaitingForActivation loop has no bounded timeout, and after a user dismisses the banner during a hung activation, no in-app signal or recovery returns. This is pre-existing but the PR explicitly tests that no action is offered during this phase without adding a bound, making it a confirmed design choice that needs a clear answer -- either a timeout in the loop or evidence that the activation layer already bounds this phase -- before this behavior is treated as finished.

Follow-up on the activation-wait and timing questions:

• The activation executors and the wait-until-finished behavior are unchanged by fix(native): recover unattended sync helper setup #681. The helper client has a 30-minute reply timeout, but that reports an unknown result; it does not terminate the detached activation. The shared lock continues to prevent overlapping system changes. Restarting nixmac does not stop that runner. The existing recovery boundary for a truly hung activation is a reboot, after inspecting the build's Console output. Adding safe cancellation or an execution deadline belongs in the activation layer, outside this presentation change.
• The 10-second threshold was already present as HELPER_GRACE_MS. This PR moves that display policy into shared state and shows neutral progress immediately. It is not an execution timeout or a measured p90. The native smoke evidence does not establish a representative p90, and this PR does not claim one.
• One title introduced here was too narrow: a running activation can come from a build or restore, not only unattended sync. Fixed in 9383d71: the banner now says “Waiting for the current build or restore to finish.” All 23 existing repair tests pass.

Source: helper wait behavior, client reply timeout.

Follow-up is blocked only on #682's current Danger run, which is queued with no ARC runner assigned. The previous metadata run waited 20 minutes before the status edit superseded it. CI maintainers need to inspect ARC capacity/dispatch, get the current run executed, and check its coverage and Danger results. Validation remains incomplete until then.

Heads are unchanged: #681 9383d7139b6b50bc7fa0f27bd55e55965fc26708; #682 6cda456b9dc420a267166f715206b056c88f00d5.

• Both #681 build and #682 build passed Linux/macOS Rust tests, strict Clippy, frontend unit tests (410/460), signing, notarization and portability. Evaluate passed on both. #681's metadata-triggered Danger run passed.
• Storybook workflows completed, but both retain 24 inherited Evolve snapshot failures and the lucide-react error generating baseline images. The five-image galleries contain only after images. No snapshots were accepted.
• Codex completed on both heads without new findings. Prelint's feat(native): add menu-bar popover and background attention #682 code review skipped unchanged files; its neutral product-review result means the review limit was reached. Danger's thin-router test warning has existing lifecycle and IPC behavior coverage. No new scoped regression was found.

Both CI checklists are updated, with #682 explicitly incomplete and native evidence limits preserved. No source edits, pushes or merges. Human review remains requested from czxtm, scottmcmaster and arximboldi.

Confidence Score: 4/5

[Medium impact] Recovers unattended sync helper setup with new retry action.

Fix stale helper-row publication and satisfy the shared-error requirement before merging.

Findings

1.  Old status hides newer failure ▶
2.  Retry bypasses shared diagnostics ▶

Summary

Repairs first-time helper registration and adds Retry without changing the saved Enable/Disable choice.

• Separates approval, setup, active-work waits, manual action, and failure into typed states.
• Keeps helper action guidance in the live permission row and adds failure details to opted-in feedback.
• The concurrency fix still lets an older loop publish Ready after a newer failure.
• The new Retry error must use the required shared diagnostics.

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Observe or user action] --> B[Read registration and saved choice]
  B --> C{No framework record?}
  C -->|Yes| D[Check bundled definition]
  C -->|No| E[Follow saved choice]
  D --> E
  E --> F{Result}
  F -->|Registered| G[Authenticate helper and check build]
  F -->|Approval or active work| H[Wait and check again]
  F -->|Failed| I[Show setup then recovery]
  G --> J[Publish typed permission row]
  H --> J
  I --> J
  J --> K[Permissions panel and repair banner]

_{Reviews (1) · Last reviewed commit: "fix(native): clarify active build helper..." · Reviewed by Greptile}

Rebuilt on main (1492663) as two commits: the backend (7ae7c85) and the UI (4e5f802).

• Registration policy: adopted main's explicit-Enable policy. Observation returns RegistrationUnavailable and never registers. helperRetry reads the stored decision after taking the slot: a stored Enable runs Invocation::Enable and a stored Disable retries the removal. An unset decision is handled exactly like an observation, so Retry never registers a missing helper, though it can still adopt an existing registration the same way an observation does. Retry never opens System Settings. The tests that expected background re-registration are gone.
• Bundle validation: check_service_definition and ServiceDefinitionError are dropped; only main's validate_bundled_definition remains.
• Stale Ready race: drive_with_clock skips a report whose generation changed during the pass and observes again instead of exiting. If an action starts while a row is being published, the loop re-observes right away. a_stale_ready_row_never_hides_a_newer_failed_action asserts on the published rows. A start request that arrives while the loop is ending is recorded and served, and each pass acknowledges the requests it serves.
• RegistrationUnavailable: mapped explicitly to the Failed phase, which offers Retry. The debug_assert! chain is now an exhaustive match, so a new report variant does not compile until it is classified.
• Removal: while a stored Disable is in progress, waiting on an activation, or failed, the row uses removal wording and offers no Enable; a failed removal offers Retry.
• Retry diagnostics: failures from the panel and the repair banner report through nostics as HELPER_E001.
• Split: kept as one PR with the backend and UI as separate commits. Merging the backend alone would leave main showing "Try again" copy with no Retry button.
• Linear: ENG-1447.

Local test results on the new head are in the description. The signed-app helper smoke on a real Mac has not been rerun on this head. #682 will be restacked once this lands.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
1.2% Duplication on New Code

See analysis details on SonarQube Cloud